Skip to content

Remove duplication description on document #3713

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Remove duplication description on document #3713

wants to merge 1 commit into from

Conversation

@adonaiohesed
Copy link

@adonaiohesed adonaiohesed commented May 3, 2019

Remove duplication description of ClientCertificate.

@adonaiohesed
Remove duplication description on document
a49ce82 
Remove duplication description of `ClientCertificate`.
@codebrain
Copy link
Contributor

codebrain commented May 6, 2019

@adonaiohesed - thanks for your commit.

In order to accept this commit we ask that you sign our CLA:
https://www.elastic.co/contributor-agreement

Please also note that the file you are editing is generated from a C# file as per the file header:

////
IMPORTANT NOTE
==============
This file has been generated from https://github.com/elastic/elasticsearch-net/tree/6.x/src/Tests/Tests/ClientConcepts/Connection/ConfigurationOptions.doc.cs. 
If you wish to submit a PR for any spelling mistakes, typos or grammatical errors for this file,
please modify the original csharp file found at the link and submit the PR with that change. Thanks!
////

Could you please include the change to this file as well?

Thanks!

@russcam
Copy link
Contributor

russcam commented May 7, 2019

I think the change to remove the duplication is more involved. The two headers relate to the following two methods:

elasticsearch-net/src/Elasticsearch.Net/Configuration/ConnectionConfiguration.cs

Lines 484 to 496 in 6521087

/// <summary>
/// Use the following certificate to authenticate all HTTP requests. You can also set them on individual
/// request using <see cref="RequestConfiguration.ClientCertificates" />
/// </summary>
public T ClientCertificate(X509Certificate certificate) =>
Assign(new X509Certificate2Collection { certificate }, (a, v) => a._clientCertificates = v);
/// <summary>
/// Use the following certificate to authenticate all HTTP requests. You can also set them on individual
/// request using <see cref="RequestConfiguration.ClientCertificates" />
/// </summary>
public T ClientCertificate(string certificatePath) =>
Assign(new X509Certificate2Collection { new X509Certificate(certificatePath) }, (a, v) => a._clientCertificates = v);

The method parameters are not currently included in the documentation, but we could consider adding them to disambiguate?

@adonaiohesed
Copy link
Author

adonaiohesed commented May 7, 2019 via email

codebrain added a commit that referenced this pull request May 28, 2019
@codebrain
Rework of #3713 - thanks @adonaiohesed
39cb86a
@codebrain
Copy link
Contributor

codebrain commented May 28, 2019

Closed this PR and submitted 39cb86a with the change.

Credited the original author, thanks again @adonaiohesed

@codebrain codebrain closed this May 28, 2019
codebrain added a commit that referenced this pull request May 28, 2019
@codebrain
Rework of #3713 - thanks @adonaiohesed
1062956
codebrain added a commit that referenced this pull request May 28, 2019
@codebrain
Rework of #3713 - thanks @adonaiohesed
bf728fe
@codebrain
Copy link
Contributor

codebrain commented May 28, 2019

Ported to master and 7.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@adonaiohesed @codebrain @russcam